Add OIDC authentication customizer for GitLab#1052
Conversation
|
Tagging for awareness: |
|
@nscuro Would you mind doing a preliminary review and letting me know if this implementation/direction needs adjustment? Thanks! |
nscuro
left a comment
There was a problem hiding this comment.
Looks good so far. I think concurrency control of the task will become interesting.
The workflow orchestration functionality we're currently working on will provide the ability to serialize based on a concurrency group, similar to what GitHub Actions has. So this undertaking will become easier in the future, but until then we need another solution.
|
|
||
| import static org.dependencytrack.model.ConfigPropertyConstants.GITLAB_ENABLED; | ||
|
|
||
| public class GitLabSyncTask implements LoggableSubscriber { |
There was a problem hiding this comment.
I presume we need a way to serialize execution of these tasks? If two or more users with overlapping GitLab project accesses login in close succession, we will encounter race conditions.
Before we go into the how (there are multiple options), can you think of any property/properties of the OIDC profile that can act as event key? i.e. what would you use as concurrency group?
There was a problem hiding this comment.
Nothing in the OIDC profile comes to mind. The only thing I can think of is maybe the groups claim, if they can be locked individually, but that doesn't give us full GitLab project paths.
The name of each GitLab project as it is read from the GitLab API response can be used as well, but that isn't available in the profile
|
@nscuro We noticed that changes in the alpine schema aren't yet reflected in hyades or the apiserver, so we created a changeset that seems to work. The changeset is now pushed to this PR. Do we need to open a PR in hyades to update schema.sql or anything? /cc @pkwiatkowski1 |
Leave it for now. We will add that migration in a separate PR anyway, since it involves a bit more non-SQL migration logic. You can keep the migration in your PR so you can test, but will likely need to rebase later down the road. |
|
@nscuro Just wanted to see if you had any ideas on a couple of bugs we're seeing. It looks like the event is somehow firing off twice, and getting a weird error about classes not being found in the CLASSPATH. For example: I'll continue debugging, just seeing if anything stands out as immediately obvious to you. Thanks! |
|
@jhoward-lm For the redundant invokation, see my comment here: stevespringett/Alpine#755 (comment)
Admittedly not sure, I'd have to checkout the code and see if I can reproduce it. |
|
@nscuro Here is the full stack trace if it helps Full error output
|
|
@nscuro In |
|
Adding
So either there's a bug in the ORM, or perhaps you have some changes locally that are not included in this PR? |
| for (String group : toCreate) { | ||
| LOGGER.debug("Creating project " + group); | ||
|
|
||
| Project existingProject = qm.getProject(group, null); |
There was a problem hiding this comment.
I can't find any place where the qm being used here is set. The code should currently throw a NPE. Is there some code you haven't yet pushed?
There was a problem hiding this comment.
Yes I have some local code that sets the qm and some other fixes. We think the use of UnblockedEvent might have something to do with the Classifier not being found. I'll push what I have shortly
fa505bb to
6cad00c
Compare
29f5a81 to
cc785c8
Compare
Bumps debian from `b5ace51` to `5724d31`. --- updated-dependencies: - dependency-name: debian dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
Some legacy code ported over from v4 still uses `DbUtil#isPostgreSQL` checks to determine if certain SQL syntax can be used. Since our move to Liquibase, `DbUtil` has not been initialized anymore, and hence always returned `false` for the aforementioned check. Ultimately, usages of `DbUtil` should be removed entirely. Signed-off-by: nscuro <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
Bumps `lib.net.javacrumbs.shedlock.version` from 6.2.0 to 6.3.0. Updates `net.javacrumbs.shedlock:shedlock-provider-jdbc` from 6.2.0 to 6.3.0 Updates `net.javacrumbs.shedlock:shedlock-provider-jdbc-internal` from 6.2.0 to 6.3.0 --- updated-dependencies: - dependency-name: net.javacrumbs.shedlock:shedlock-provider-jdbc dependency-type: direct:production update-type: version-update:semver-minor - dependency-name: net.javacrumbs.shedlock:shedlock-provider-jdbc-internal dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: Jonathan Howard <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: Jonathan Howard <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
Analogue to DependencyTrack/hyades#1672 Signed-off-by: nscuro <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
A mix-up of `"V"."VULNID" != ANY(:vulnIdsToExclude)` and `"V"."VULNID" != ALL(:vulnIdsToExclude)` caused all but one Snyk vulnerability to be suppressed for a component. https://stackoverflow.com/a/11730845 Signed-off-by: nscuro <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: nscuro <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
…1064) Signed-off-by: Allen Shearin <[email protected]>
Bumps `lib.testcontainers.version` from 1.20.4 to 1.20.5. Updates `org.testcontainers:kafka` from 1.20.4 to 1.20.5 - [Release notes](https://github.com/testcontainers/testcontainers-java/releases) - [Changelog](https://github.com/testcontainers/testcontainers-java/blob/main/CHANGELOG.md) - [Commits](testcontainers/testcontainers-java@1.20.4...1.20.5) Updates `org.testcontainers:postgresql` from 1.20.4 to 1.20.5 - [Release notes](https://github.com/testcontainers/testcontainers-java/releases) - [Changelog](https://github.com/testcontainers/testcontainers-java/blob/main/CHANGELOG.md) - [Commits](testcontainers/testcontainers-java@1.20.4...1.20.5) --- updated-dependencies: - dependency-name: org.testcontainers:kafka dependency-type: direct:development update-type: version-update:semver-patch - dependency-name: org.testcontainers:postgresql dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
Bumps [org.apache.maven.plugins:maven-clean-plugin](https://github.com/apache/maven-clean-plugin) from 3.4.0 to 3.4.1. - [Release notes](https://github.com/apache/maven-clean-plugin/releases) - [Commits](apache/maven-clean-plugin@maven-clean-plugin-3.4.0...maven-clean-plugin-3.4.1) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-clean-plugin dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: Jonathan Howard <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
* roleQueryManagerTest Signed-off-by: Philippe <[email protected]> * update role query manager test Signed-off-by: Philippe <[email protected]> * fix RoleQueryManagerTest and the USER_PROJECT_EFFECTIVE_PERMISSIONS view Signed-off-by: Philippe <[email protected]> * add remaining RoleQueryManager tests Signed-off-by: Philippe <[email protected]> * add TODO to failing test Signed-off-by: Philippe <[email protected]> * fix formatting Signed-off-by: Allen Shearin <[email protected]> * fix RoleQueryManagerTest.testGetRole Signed-off-by: Philippe <[email protected]> --------- Signed-off-by: Philippe <[email protected]> Signed-off-by: Allen Shearin <[email protected]> Co-authored-by: Allen Shearin <[email protected]>
…erver into pull-upstream-main Signed-off-by: Jonathan Howard <[email protected]>
Signed-off-by: Jonathan Howard <[email protected]>
Signed-off-by: Jonathan Howard <[email protected]>
…erver into roles-consolidate-user-tables
Signed-off-by: Jonathan Howard <[email protected]>
…te-user-tables Roles consolidate user tables
Signed-off-by: Jonathan Howard <[email protected]>
Signed-off-by: Jonathan Howard <[email protected]>
…apiserver into gitlab-integration Signed-off-by: Jonathan Howard <[email protected]>
Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: Allen Shearin <[email protected]>
Co-authored-by: jhoward-lm <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: Allen Shearin <[email protected]>
add gitlab integration state toggle
Signed-off-by: Allen Shearin <[email protected]>
…nto add-roles-model Signed-off-by: Allen Shearin <[email protected]>
Signed-off-by: Jonathan Howard <[email protected]>
Signed-off-by: Jonathan Howard <[email protected]>
regenerate jooq with role tables
…apiserver into gitlab-integration
…apiserver into gitlab-integration Signed-off-by: Jonathan Howard <[email protected]>
…erver into gitlab-integration
|
Closing as superseded by #1325 |
Description
This PR adds an implementation of the
OidcAuthenticationCustomizerservice provider interface specific to GitLab. It is meant to synchronize a user's projects and roles/max access levels per project within GitLab to a Hyades instance.Depends on stevespringett/Alpine#755 and #1069.
Note
WIP: a rough to-do list with outstanding items is in a comment block in
GitLabSyncTask.javaAddressed Issue
Closes DependencyTrack/hyades#1632
Additional Details
This PR is a request for early review while still in development in order to change direction if necessary.
Checklist
This PR fixes a defect, and I have provided tests to verify that the fix is effective